Repository navigation
fix(otel): await server.stop(true) to release port in stopOtelReceiver - #1177
Conversation
The push-event CI intermittently failed on `otel-receiver > POST /v1/logs handles empty payload` with EADDRINUSE because `stopOtelReceiver()` called `server.stop()` without awaiting the returned Promise. The server state was nulled synchronously, but the underlying TCP port lingered. When the next test's `beforeEach` picked a random port from the 7000-slot window (57000-63999) and `startOtelReceiver()` raced against the unreleased port, start silently returned false (caught as non-fatal EADDRINUSE), leaving `isOtelReceiverRunning()` true but the server unreachable — so downstream fetch() calls returned 0 or hit the wrong route. Fix: - Make `stopOtelReceiver()` async, await `server.stop(true)` (force close). - Update the two callers in `src/genie.ts` (finally blocks) to await. - Update `afterEach` in otel-receiver.test.ts to await stop. - Add `expect(started).toBe(true)` assertions after every `await startOtelReceiver()` call so any future silent-false failure surfaces immediately instead of cascading into misleading HTTP errors. Validated: 5 consecutive local runs of otel-receiver.test.ts pass; full `bun test` (2581 tests) passes; `bun run typecheck` clean.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request converts stopOtelReceiver into an asynchronous function that awaits the server's shutdown, ensuring the listening port is fully released to prevent EADDRINUSE errors during tests. Corresponding updates were made to src/genie.ts and src/lib/otel-receiver.test.ts to await the function and handle its promise. Feedback was provided regarding a potential race condition in stopOtelReceiver, suggesting that the server reference should be nulled before awaiting the stop operation to ensure the system state is updated immediately.
| export async function stopOtelReceiver(): Promise<void> { | ||
| if (server) { | ||
| server.stop(); | ||
| await server.stop(true); | ||
| server = null; | ||
| } | ||
| } |
There was a problem hiding this comment.
There is a potential race condition in the current implementation of stopOtelReceiver. Because server = null is executed after the await, any calls to isOtelReceiverRunning() or startOtelReceiver() that occur while the stop operation is pending will see the server as still active. This could lead to situations where the system incorrectly assumes the receiver is running when it is actually being shut down.
It is safer to capture the server reference and null the module-level variable immediately before awaiting the stop operation. This also makes the function naturally idempotent if called multiple times concurrently.
| export async function stopOtelReceiver(): Promise<void> { | |
| if (server) { | |
| server.stop(); | |
| await server.stop(true); | |
| server = null; | |
| } | |
| } | |
| export async function stopOtelReceiver(): Promise<void> { | |
| const s = server; | |
| if (s) { | |
| server = null; | |
| await s.stop(true); | |
| } | |
| } |
Summary
Fixes the intermittent push-event CI failure on
otel-receiver > POST /v1/logs handles empty payload.Root cause:
stopOtelReceiver()calledserver.stop()without awaiting the returned Promise. State was nulled synchronously, but the TCP port lingered. The next test'sbeforeEachpicked a random port from the 7000-slot window (57000-63999); when the window was wide enough the previous suite's test and the next start raced on the same port,startOtelReceiver()hit EADDRINUSE, caught it as non-fatal, and silently returnedfalse.isOtelReceiverRunning()stayedtrue(server variable still set from a prior run in pathological ordering), so downstreamfetch()calls reached the wrong endpoint and returned unexpected status codes.Fix:
stopOtelReceiver()is nowasyncand awaitsserver.stop(true)(force close) — seesrc/lib/otel-receiver.ts:374-386.src/genie.ts:700,711callers nowawait stopOtelReceiver().catch(() => {})in their finally blocks.src/lib/otel-receiver.test.tsafterEachawaits stop.await startOtelReceiver()call site getsexpect(started).toBe(true)so future silent-false regressions surface immediately.Test plan
bun test src/lib/otel-receiver.test.ts— 9/9 pass (5 consecutive runs)bun test— 2581/2581 passbun run typecheck— cleanbun run lint— no new warnings on changed filesContext
Blocks the auto-version bump that publishes PR #1175 (topology fix for auto-team-of-one) to the
@automagik/genie@nextdist-tag. Once this merges and CI goes green, the publish pipeline resumes and the team-of-one-auto-create wish can complete G-C verification.